Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hold event loop during main() #841

Merged
merged 1 commit into from
Dec 7, 2023
Merged

Hold event loop during main() #841

merged 1 commit into from
Dec 7, 2023

Conversation

cowboyd
Copy link
Member

@cowboyd cowboyd commented Dec 7, 2023

Motivation

When you run the following program:

import { main } from "effection";

await main(function*() {
  yield* suspend();
});

You get this error

error: Top-level await promise never resolved

Because there is literally nothing on the event loop which means that there is nothing that could ever resolve the promise, and yet we should still be suspended. Actually, there is some state in that main() registers SIGINT handlers, but both Deno and Node say that SIGINT doesn't "count" because the pre-structured-concurrency state of the art is to call process.exit() on interrupt and shoot the process in the heart. Effection on the other hand removes the sigint handler, so if that "counted" the main() would hold the process open, but release it after the signal handlers had run.

Approach

This just holds the process open by installing a setInterval that fires every 2^30 milliseconds (about ten years). It is removed when main is finished.

Possible Drawbacks or Risks

One thing that has occured to me which I'm not sure about is that run() will still exhibit the same behavior. In other words:

await run(suspend);

Will complain that the run() promise has not resolved, but that the event loop is exhausted. That feels a bit asymmetric and suprising which is not great. We could make suspend() itself hold the event loop with the long interval, although that worries me that every single suspend() operation would install a dummy interval. Is it a tough look to have hundreds, or perhaps even thousands of dummy intervals?

Alternate Designs

Another option would be to make any Frame that does not have a parent (aka root frame) of which there is generally only one, hold the event loop, until its destruction. That would allow it to work with a bare run(), but not install a setTimeout every time a suspend() operation is encountered. The number of dummy intervals would be equal to the number of root frames (most of the time one).

Learning

When you run the following program:

```js
import { main } from "effection";

await main(function*() {
  yield* suspend();
});
```

You get this error

```
error: Top-level await promise never resolved
```

Because there is literally nothing on the event loop which means that
there is nothing that _could_ ever resolve the promise, and yet we
should still be suspended. Actually, there _is_ some state in that
`main()` registers `SIGINT` handlers, but both Deno and Node say that
`SIGINT` doesn't "count" because the state of the art is to call
process.exit() on interrupt and shoot the process heart. Effection on
the other hand removes the sigint handler, so if that "counted" the
`main()` would hold the process open.

This just holds the process open by installing a `setInterval` that fires
every 2^30 milliseconds (about ten years). It is removed when main is finished.

One thing that has occured to me which I'm not sure about is that `run()`
will still exhibit the same behavior. In other words:

```js
await run(suspend);
```

Will complain that the `run()` promise has not resolved, but that
the event loop is exhausted. That feels a bit asymmenric and suprising
which is not great. We could make `suspend()` itself hold the event
loop with the long interval, although that worries me that every
single `suspend()` operation would install a dummy interval. Is it a
tough look to have hundreds, or perhaps even thousands of dummy intervals?

Another option would be to make any Frame that does not have a
parent (aka root frame) of which there is generally only one, hold the
event loop, until its destruction. That would allow it to work with a
bare `run()`, but not install a `setTimeout` every time a suspend()
operation is encountered. The number of dummy intervals would be equal
to the number of root frames (most of the time one).
@neurosnap
Copy link
Collaborator

I don't think we need to accommodate this for run() as it feels like an edge-case and mostly for demonstration purposes, right?

@cowboyd
Copy link
Member Author

cowboyd commented Dec 7, 2023

@neurosnap It is mostly edgy, but I do think in my heart of heart's that this should work, and so the fact that it would only work in main does bother me a bit.

import { run } from "effection";

await run(function*() {
  yield* action(function*(resolve) {
    try {
      process.on("SIGINT", resolve);
      yield* suspend();
    } finally {
      process.off("SIGINT", resolve);
    }
  });
});

@cowboyd
Copy link
Member Author

cowboyd commented Dec 7, 2023

I guess we can revisit this if it becomes a problem since it won't break anything to have both work in the future.

@cowboyd cowboyd merged commit ae6d9c7 into v3 Dec 7, 2023
3 checks passed
@cowboyd cowboyd deleted the main-holds-event-loop branch December 7, 2023 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants